Skip to content

Conversation

@mdelapenya
Copy link
Member

What does this PR do?

Use the Run function in vault

Why is it important?

Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.

Related issues

This commit migrates the vault module to use the new testcontainers.Run() API.

The main changes are:
- Use testcontainers.Run() instead of testcontainers.GenericContainer()
- Convert to moduleOpts pattern with functional options
- Use WithExposedPorts, WithHostConfigModifier, WithWaitStrategy, WithEnv
- WithToken uses WithEnv for setting root token env vars
- WithInitCommand uses WithAdditionalWaitStrategy to append exec wait

Tests: 10 tests, 89.5% coverage

Ref: testcontainers#3174

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mdelapenya mdelapenya requested a review from a team as a code owner October 9, 2025 15:25
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 9, 2025
@netlify
Copy link

netlify bot commented Oct 9, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit ff0c701
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68e7d94ec792eb0008d0debc
😎 Deploy Preview https://deploy-preview-3443--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Summary by CodeRabbit

  • Refactor

    • Simplified container setup using a modular options-based configuration without changing user-facing behavior.
  • Bug Fixes

    • Preserved necessary capabilities to prevent potential memory lock issues, improving container stability.
    • Improved startup reliability with a consolidated wait and environment configuration.
  • Chores

    • Clarified error messages when starting the Vault container.

Walkthrough

Refactors Vault module to use testcontainers.Run with modular ContainerCustomizer options. Replaces direct ContainerRequest mutations for env, wait strategy, exposed ports, and host config. Updates error messages. Adjusts WithToken and WithInitCommand to return options (WithEnv, WithAdditionalWaitStrategy). Preserves CAP_IPC_LOCK via HostConfigModifier.

Changes

Cohort / File(s) Summary
Vault module: option-based Run refactor
modules/vault/vault.go
Migrate from explicit ContainerRequest/GenericContainer to testcontainers.Run with moduleOpts (WithExposedPorts, WithEnv, WithWaitStrategy, WithHostConfigModifier). Update error to "run vault: %w". Change WithToken to return WithEnv(...). Change WithInitCommand to return WithAdditionalWaitStrategy(wait.ForExec(cmd)). Ensure CAP_IPC_LOCK remains set.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as Caller
  participant V as vault.Run(...)
  participant TC as testcontainers.Run
  participant D as Docker Engine

  U->>V: Run(ctx, img, opts...)
  Note right of V: Build moduleOpts<br/>- WithExposedPorts<br/>- WithEnv<br/>- WithWaitStrategy<br/>- WithHostConfigModifier (CAP_IPC_LOCK)
  V->>TC: Run(ctx, img, moduleOpts+opts)
  TC->>D: Create & start container
  alt start succeeds
    D-->>TC: Container started
    TC-->>V: Container handle
    V-->>U: Return container
  else error
    D-->>TC: Error
    TC-->>V: Error
    V-->>U: wrap "run vault: %w"
  end

  rect rgba(230,245,255,0.5)
  Note over U,V: Option hooks
  U->>V: WithToken(token)
  V-->>U: returns WithEnv("VAULT_TOKEN"=token)
  U->>V: WithInitCommand(cmd)
  V-->>U: returns WithAdditionalWaitStrategy(wait.ForExec(cmd))
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

I hop through opts with nimble feet,
From rigid reqs to runs so neat.
Env and waits now neatly spun,
A carrot-colored CAP_IPC_LOCK won.
In burrows built with tidy fun—
Vault awakens, job well done! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly conveys the primary change of migrating the vault module to use the Run function, matching the main refactoring described in the changeset. It is concise, specific, and clearly tied to the core update without extraneous detail.
Description Check ✅ Passed The description clearly outlines what the PR does—using the Run function in the vault module—explains its importance by referencing consistency and API improvements, and even links the related issue, so it directly relates to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c64a46 and 6f794e5.

📒 Files selected for processing (1)
  • modules/vault/vault.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/vault/vault.go (3)
options.go (6)
  • ContainerCustomizer (22-24)
  • WithExposedPorts (454-459)
  • WithHostConfigModifier (88-94)
  • WithWaitStrategy (366-368)
  • WithEnv (75-85)
  • WithAdditionalWaitStrategy (371-373)
wait/http.go (1)
  • ForHTTP (149-151)
wait/exec.go (1)
  • ForExec (71-73)
🔇 Additional comments (6)
modules/vault/vault.go (6)

31-40: LGTM! Clean refactoring to modular options.

The migration to the options-based approach is well-executed. All necessary configuration (exposed ports, CAP_IPC_LOCK capability, HTTP health check, and VAULT_ADDR environment variable) is correctly preserved using the appropriate ContainerCustomizer functions.


42-42: LGTM! Correct option composition.

The options are properly combined with append(moduleOpts, opts...), ensuring module defaults are applied first while allowing user-provided options to override them.


44-46: LGTM! Defensive programming.

The nil check before wrapping ensures a non-nil VaultContainer can be returned even when an error occurs, which is good defensive programming practice.


49-49: LGTM! Improved error clarity.

The error message change from "generic container" to "run vault" is more specific and better reflects the context of the operation.


58-62: LGTM! Correct delegation to WithEnv.

The refactoring correctly delegates to testcontainers.WithEnv, which handles map initialization and follows the composable options pattern. The token environment variables are properly configured.


74-74: LGTM! Correct use of WithAdditionalWaitStrategy.

Using WithAdditionalWaitStrategy instead of WithWaitStrategy is critical here—it appends the exec wait strategy to the existing HTTP health check on line 36 rather than replacing it. This correctly preserves the multi-wait behavior.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdelapenya mdelapenya merged commit d0b6154 into testcontainers:main Oct 9, 2025
17 checks passed
@mdelapenya mdelapenya deleted the use-run-claude-vault branch October 9, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant